Feature/cstackex 265: Netapp ONTAP iscsi provisioning using lun serial number instead of logical unit number by piyush5netapp · Pull Request #114 · NetApp/cloudstack

piyush5netapp · 2026-10-08T04:16:13Z

Description

This PR...

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

All provisioning workflows has been tested.

virsh domblklist i-2-73-VM
Target Source

vda /dev/disk/by-id/wwn-0x600a098078304d2d383f2f6b45734c63
vdb /dev/disk/by-id/wwn-0x600a098078304d2d383f2f6b45734c64
hdc -
hdd -
ls -la /dev/disk/by-id/
total 0
lrwxrwxrwx 1 root root 9 Oct 14 15:56 scsi-3600a098078304d2d383f2f6b45734c63 -> ../../sdc
lrwxrwxrwx 1 root root 10 Oct 14 15:56 scsi-3600a098078304d2d383f2f6b45734c63-part1 -> ../../sdc1
lrwxrwxrwx 1 root root 10 Oct 14 15:56 scsi-3600a098078304d2d383f2f6b45734c63-part2 -> ../../sdc2
lrwxrwxrwx 1 root root 9 Oct 14 15:56 scsi-3600a098078304d2d383f2f6b45734c64 -> ../../sdd
lrwxrwxrwx 1 root root 9 Oct 14 15:56 wwn-0x600a098078304d2d383f2f6b45734c63 -> ../../sdc
lrwxrwxrwx 1 root root 10 Oct 14 15:56 wwn-0x600a098078304d2d383f2f6b45734c63-part1 -> ../../sdc1
lrwxrwxrwx 1 root root 10 Oct 14 15:56 wwn-0x600a098078304d2d383f2f6b45734c63-part2 -> ../../sdc2
lrwxrwxrwx 1 root root 9 Oct 14 15:56 wwn-0x600a098078304d2d383f2f6b45734c64 -> ../../sdd

How did you try to break this feature and the system with this change?

github-actions · 2026-10-08T05:17:44Z

🔴 Test Coverage Grade: `D` — Marginal

Metric	Value
Line coverage	24.74%
Branch coverage	18.86%

Grade Scale

Grade	Line Coverage	Meaning
🟢 A	≥ 80%	Excellent - this code sleeps well at night 😴
🟡 B	60-79%	Good - almost there, don't stop now 😉
🟠 C	40-59%	Acceptable - your code is wearing a seatbelt, but no airbags 😬
🔴 D	20-39%	Marginal - boldly shipping where no test has gone before 🖖
⛔ F	< 20%	Failing - tests? what tests? 🔥

Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run

github-actions · 2026-10-08T11:25:54Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

github-actions · 2026-10-12T05:29:49Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

sandeeplocharla · 2026-10-12T15:48:17Z

                OntapStorageConstants.SVM_DOT_NAME, svmName,
                OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName
        );
        String lunNumber = getLogicalAccess(getMap);


Do change this code according to fetch the WWID instead.

As discussed, this change required a refactoring code changes in ontap plugin code as now logicalaccess is not lun number and we have some dead code change. https://netapp.atlassian.net/browse/CSTACKEX-349

sandeeplocharla · 2026-10-12T15:50:25Z

+     *
+     * @return the serial number, or null if the LUN no longer exists on the array
+     */
+    public String getLunSerialNumber(String svmName, String lunName) {


This method might not be needed, as earlier LUN was fetched in ensureLunMapped, so the same implementation would be present there. Just change the query param accordingly.

Same as https://netapp.atlassian.net/browse/CSTACKEX-349. Also , lun-map API does not return lun serial number and we need to call get Lun API.

sandeeplocharla · 2026-10-12T15:52:38Z

+    /**
+     * NetApp's IEEE OUI, which prefixes the NAA identifier of every ONTAP LUN.
+     *
+     * <p>A LUN's WWID is this followed by the hex of its 12-character serial number, giving the


Try to restrict the explanation to 1-2 lines highlighting the structure of the path

sandeeplocharla · 2026-10-12T15:53:52Z

    }

+    @Override
+    public boolean zoneWideVolumesAvailableWithoutClusterMotion(){


Maybe its better to raise a separate PR for this, as we would be cherry-picking the commits for upstream

We discussed in planning meeting, Piyush will take this change to upstream and add the compute to compute testing details also

sandeeplocharla · 2026-10-12T15:56:27Z

+ *
+ * So {@code OntapPrimaryDatastoreDriver} writes volume paths as {@code /<targetIQN>/<lunWwid>} and
+ * this adaptor reports devices as {@code /dev/disk/by-id/scsi-3<lunWwid>}. The WWID is derived from
+ * the LUN's own inquiry data (VPD page 0x83), so it is identical on every host and the domain XML


Most of this comment may not be necessary for future, so, consider pruning this to explain the adaptor in brief, instead.

sandeeplocharla · 2026-10-12T16:02:21Z

+     * udev's scsi_id builtin prepends the NAA designator type to the WWID, so a LUN whose WWID is
+     * {@code 600a0980...} is published as {@code scsi-3600a0980...}.
+     */
+    private static final String BY_ID_SCSI_PREFIX = "/dev/disk/by-id/scsi-3";


For future reference, add a comment here or in our spec that /dev/disk/by-id/scsi-3 is set here as ONTAP uses NAA and 600a098... is the NetApp OUI vendor prefix 00:a0:98

sandeeplocharla · 2026-10-12T16:15:34Z

+        return null;
+    }
+
+    private boolean createIscsiNode(String host, int port, String iqn, String volumePath) {


Add null checks for all the variables

iscsi session will throw error and it is handled automatically.

sandeeplocharla · 2026-10-12T16:21:58Z

-            KVMStoragePool destPool, int timeout, byte[] passphrase) {
-        throw new UnsupportedOperationException("Creating a disk from a template is not yet supported for this configuration.");
+    private void waitForDeviceToDisappear(String devicePath) {
+        for (int attempt = 0; attempt < DEVICE_WAIT_TRIES && getDeviceSize(devicePath) > 0; attempt++) {


Is there any reason for 15s wait period for devices to respond everywhere in this file? Or was this pulled in from iSCSIAdmAdaptor?

Its just a cushion time.And yes it was pulled from iscsi adapter

suryag1201 · 2026-10-12T19:28:37Z

 import org.apache.commons.lang3.StringUtils;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;


could u run this command
udevadm info -q symlink -n /dev/sdb
will this show both forms in that output (Id and path )?

udevadm info -q symlink -n /dev/sdc
disk/by-id/scsi-3600a098078304d2d383f2f6b45734c2f disk/by-id/wwn-0x600a098078304d2d383f2f6b45734c2f disk/by-path/ip-10.196.37.157:3260-iscsi-iqn.1992-08.com.netapp:sn.45048fd9b65111f1b106005056bd83cf:vs.3-lun-0.

it is still showing this bcz udev created with lun number also? So, it is fine?
disk/by-path/ip-10.196.37.157:3260-iscsi-iqn.1992-08.com.netapp:sn.45048fd9b65111f1b106005056bd83cf:vs.3-lun-0

suryag1201 · 2026-10-12T19:33:56Z


    @Override
-    public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {
+    public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {


please check this method: IscsiStorageCleanupMonitor, this used by-path param and if not exact match then iSCSI cleanup thread will log out live ONTAP sessions

We do not support iscsi clean flag = true for Netapp ONTAP.

suryag1201 · 2026-10-12T19:35:30Z


-        // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 -o new
-        Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
+        if (!createIscsiNode(host, port, iqn, volumePath)) {


Perform this test also, when 2 VMs are running and if u stop 1 VM what happens?
does this logout the whole session ?

No, whole session is not removed.

suryag1201 · 2026-10-12T19:46:39Z


-        physicalDisk.setSize(deviceSize);
-        physicalDisk.setVirtualSize(deviceSize);
+        if (!logoutAndForgetTarget(host, port, iqn, lunWwid)) {


If the session cannot be identified, do not log out. Remove only this SCSI device, or fail the disconnect so it can be retried. Logout should run only after the session is known and no other LUNs remain on it.

Done. taken care.

piyush5netapp · 2026-10-12T09:26:03Z

- * Must remain in {@code com.cloud.hypervisor.kvm.storage}: {@link KVMStoragePoolManager}
- * discovers adaptors only in that package. An unregistered type falls back to
- * {@link LibvirtStorageAdaptor}.
+ * Serves {@link StoragePoolType#OntapiSCSI} pools: ONTAP FlexVols exposed over iSCSI with one LUN


Check for the comment reframing

piyush5netapp · 2026-10-12T09:28:46Z

+     * Anchoring on the prefix is what stops this adaptor claiming another vendor's scsi-3 device
+     * when KVMStoragePoolManager.disconnectPhysicalDiskByPath scans every adaptor.
+     */
+    private static final Pattern LUN_WWID = Pattern.compile("600a0980[0-9a-f]{24}");


make Netapp OUI as a constant.

piyush5netapp · 2026-10-14T09:14:03Z

                OntapStorageConstants.SVM_DOT_NAME, svmName,
                OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName
        );
        String lunNumber = getLogicalAccess(getMap);


As discussed, this change required a refactoring code changes in ontap plugin code as now logicalaccess is not lun number and we have some dead code change. https://netapp.atlassian.net/browse/CSTACKEX-349

piyush5netapp · 2026-10-14T09:14:12Z

+     *
+     * @return the serial number, or null if the LUN no longer exists on the array
+     */
+    public String getLunSerialNumber(String svmName, String lunName) {


Same as https://netapp.atlassian.net/browse/CSTACKEX-349. Also , lun-map API does not return lun serial number and we need to call get Lun API.

piyush5netapp · 2026-10-14T09:14:16Z

+    /**
+     * NetApp's IEEE OUI, which prefixes the NAA identifier of every ONTAP LUN.
+     *
+     * <p>A LUN's WWID is this followed by the hex of its 12-character serial number, giving the


piyush5netapp · 2026-10-14T09:15:33Z

 import org.apache.commons.lang3.StringUtils;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;


udevadm info -q symlink -n /dev/sdc
disk/by-id/scsi-3600a098078304d2d383f2f6b45734c2f disk/by-id/wwn-0x600a098078304d2d383f2f6b45734c2f disk/by-path/ip-10.196.37.157:3260-iscsi-iqn.1992-08.com.netapp:sn.45048fd9b65111f1b106005056bd83cf:vs.3-lun-0.

piyush5netapp · 2026-10-14T09:22:27Z


    @Override
-    public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {
+    public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {


We do not support iscsi clean flag = true for Netapp ONTAP.

piyush5netapp · 2026-10-14T09:25:00Z


-        // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 -o new
-        Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, logger);
+        if (!createIscsiNode(host, port, iqn, volumePath)) {


No, whole session is not removed.

piyush5netapp · 2026-10-14T09:25:14Z


-        physicalDisk.setSize(deviceSize);
-        physicalDisk.setVirtualSize(deviceSize);
+        if (!logoutAndForgetTarget(host, port, iqn, lunWwid)) {


Done. taken care.

github-actions · 2026-10-14T11:57:36Z

🔴 Test Coverage Grade: `D` — Marginal

Metric	Value
Line coverage	24.85%
Branch coverage	19.01%

Grade Scale

Grade	Line Coverage	Meaning
🟢 A	≥ 80%	Excellent - this code sleeps well at night 😴
🟡 B	60-79%	Good - almost there, don't stop now 😉
🟠 C	40-59%	Acceptable - your code is wearing a seatbelt, but no airbags 😬
🔴 D	20-39%	Marginal - boldly shipping where no test has gone before 🖖
⛔ F	< 20%	Failing - tests? what tests? 🔥

Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run

github-actions · 2026-10-14T13:08:49Z

🔴 Test Coverage Grade: `D` — Marginal

Metric	Value
Line coverage	24.85%
Branch coverage	19.01%

Grade Scale

Grade	Line Coverage	Meaning
🟢 A	≥ 80%	Excellent - this code sleeps well at night 😴
🟡 B	60-79%	Good - almost there, don't stop now 😉
🟠 C	40-59%	Acceptable - your code is wearing a seatbelt, but no airbags 😬
🔴 D	20-39%	Marginal - boldly shipping where no test has gone before 🖖
⛔ F	< 20%	Failing - tests? what tests? 🔥

Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run

Srivastava, Piyush added 4 commits September 21, 2026 20:36

feature/CSTACKEX-265: Pilot commit

7322d33

feature/CSTACKEX-265: testing code

8399fcf

feature/CSTACKEX-265: Updated netapp oui check for disconnect

850f7b2

feature/CSTACKEX-265: Pulled latest from main

2b9b817

piyush5netapp requested review from rajiv-jain-netapp, sandeeplocharla and suryag1201 as code owners October 8, 2026 04:16

github-actions Bot added the status:has-conflicts label Oct 8, 2026

Srivastava, Piyush added 2 commits October 12, 2026 01:11

feature/CSTACKEX-265: udpated code

c2bb377

feature/CSTACKEX-265: Pulled latest from main

8d220ae

github-actions Bot added status:has-conflicts and removed status:has-conflicts labels Oct 11, 2026

feature/CSTACKEX-265: Pulled latest from main

f4bf026

github-actions Bot removed the status:has-conflicts label Oct 12, 2026

sandeeplocharla reviewed Oct 12, 2026

View reviewed changes

suryag1201 reviewed Oct 12, 2026

View reviewed changes

Srivastava, Piyush added 2 commits October 14, 2026 14:03

feature/CSTACKEX-265: Updated the comments

5aaae99

feature/CSTACKEX-265: Updated the comments

adce179

piyush5netapp commented Oct 14, 2026

View reviewed changes

Srivastava, Piyush added 2 commits October 14, 2026 16:14

feature/CSTACKEX-265: Updated the comments

b765cf8

feature/CSTACKEX-265: Updated UT failure

ea76804

feature/CSTACKEX-265: Updated UT failure

648668c

sandeeplocharla approved these changes Oct 14, 2026

View reviewed changes

suryag1201 approved these changes Oct 14, 2026

View reviewed changes

piyush5netapp merged commit e2a2682 into main Oct 14, 2026
12 checks passed

Conversation

piyush5netapp commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

All provisioning workflows has been tested. virsh domblklist i-2-73-VM Target Source

How did you try to break this feature and the system with this change?

Uh oh!

github-actions Bot commented Oct 8, 2026

🔴 Test Coverage Grade: D — Marginal

Grade Scale

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyush5netapp Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suryag1201 Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyush5netapp Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyush5netapp commented Oct 8, 2026 •

edited

Loading

All provisioning workflows has been tested.

virsh domblklist i-2-73-VM
Target Source

🔴 Test Coverage Grade: `D` — Marginal

piyush5netapp Oct 14, 2026 •

edited

Loading

suryag1201 Oct 14, 2026 •

edited

Loading

piyush5netapp Oct 14, 2026 •

edited

Loading

piyush5netapp Oct 14, 2026 •

edited

Loading

piyush5netapp Oct 14, 2026 •

edited

Loading

🔴 Test Coverage Grade: `D` — Marginal

🔴 Test Coverage Grade: `D` — Marginal